Skip to content

Add CBT (Changed Block Tracking) test suite - #541

Draft
Lankou66 wants to merge 8 commits into
masterfrom
gru_cbt_test
Draft

Add CBT (Changed Block Tracking) test suite#541
Lankou66 wants to merge 8 commits into
masterfrom
gru_cbt_test

Conversation

@Lankou66

@Lankou66 Lankou66 commented May 20, 2026

Copy link
Copy Markdown
Contributor

Add CBT (Changed Block Tracking) test suite

This PR implements a test suite for the CBT feature across following SR types:

  • EXT
  • LVM
  • LVMoISCSI
  • LVMoHBA
  • NFS
  • XFS
  • ZFS

@Lankou66
Lankou66 force-pushed the gru_cbt_test branch 7 times, most recently from ee415bc to 71b7668 Compare June 16, 2026 17:49
@Lankou66 Lankou66 changed the title Implement CBT Test Add CBT (Changed Block Tracking) test suite Jun 18, 2026
@Lankou66
Lankou66 marked this pull request as ready for review June 18, 2026 08:47
@Lankou66
Lankou66 requested review from a team as code owners June 18, 2026 08:47
@glehmann

Copy link
Copy Markdown
Member

@Lankou66 could you rebase on master before we review? There are a lot of conflicts

Comment thread lib/vdi.py

ImageFormat = Literal['qcow2', 'raw', 'vhd']


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a necessary change?

@stormi stormi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick look but stopped due to:

As time is precious, I'll consider reviewing once this is fixed.

Lankou66 added 8 commits June 18, 2026 12:21
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>
Signed-off-by: Goulven Riou <goulven.riou@vates.tech>

@glehmann glehmann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the commit organization, I would have preferred, to ease the review, that each commit introduce a test or a set of related tests, with the generic method in storage.py, the implementation for all the SR types, and an explanation of what it does in the commit description.

Comment thread tests/storage/storage.py
Comment on lines +566 to +567
result = host.ssh(f'test -f {log_path}', check=False, simple_output=False)
assert result.returncode == 0, f"CBT log not found at {log_path}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result = host.ssh(f'test -f {log_path}', check=False, simple_output=False)
assert result.returncode == 0, f"CBT log not found at {log_path}"
assert host.file_exists(log_path), f"CBT log not found at {log_path}"

Comment thread tests/storage/storage.py
Comment on lines +568 to +569
result = host.ssh(f'stat -c %s {log_path}', simple_output=False)
log_size = int(result.stdout.strip())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result = host.ssh(f'stat -c %s {log_path}', simple_output=False)
log_size = int(result.stdout.strip())
log_size = int(host.ssh(f'stat -c %s {log_path}').strip())

Comment thread tests/storage/storage.py
Comment on lines +577 to +579
result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False)
assert cbt_log_name in result.stdout, \
f"CBT log LV {cbt_log_name} not found in VG {vg_name}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False)
assert cbt_log_name in result.stdout, \
f"CBT log LV {cbt_log_name} not found in VG {vg_name}"
result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}')
assert cbt_log_name in result, f"CBT log LV {cbt_log_name} not found in VG {vg_name}"

Comment thread tests/storage/storage.py
Comment on lines +585 to +586
result = host.ssh(f'test -f {log_path}', check=False, simple_output=False)
assert result.returncode != 0, f"CBT log should not exist at {log_path}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result = host.ssh(f'test -f {log_path}', check=False, simple_output=False)
assert result.returncode != 0, f"CBT log should not exist at {log_path}"
assert host.file_exists(log_path), f"CBT log should not exist at {log_path}"

Comment thread tests/storage/storage.py
Comment on lines +593 to +595
result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False)
assert cbt_log_name not in result.stdout, \
f"CBT log LV {cbt_log_name} should not exist in VG {vg_name}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}', simple_output=False)
assert cbt_log_name not in result.stdout, \
f"CBT log LV {cbt_log_name} should not exist in VG {vg_name}"
result = host.ssh(f'lvs --noheadings -o lv_name {vg_name}')
assert cbt_log_name not in result, f"CBT log LV {cbt_log_name} should not exist in VG {vg_name}"

Comment thread tests/storage/storage.py


@contextmanager
def cbt_enabled(vdi: VDI) -> Generator[VDI, None, None]:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this rather be a fixture?

Fixtures has a great advantage over context manager to deal with resources: when a test fails, the debugger is started before the resources are destroyed. The teardown is executed once you exit the debugger. It helps a lot to investigate in case of failure.

This is probably also true—at least I would ask the same question—for the other context managers.

Comment thread tests/storage/storage.py
vm.disconnect_vdi(vdi)
vm.shutdown(verify=True)
second = vdi.snapshot()
stack.callback(second.destroy)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same idea as for the context manager applies here.

We have a defer fixture which does the same thing, but ensures that the resources are preserved when entering the debugger.

Comment thread tests/storage/storage.py
for i in range(len(snapshots) - 1):
assert verify_changed_blocks_detected(snapshots[i], snapshots[i + 1])
logging.info(f"Changes detected: snap{i} -> snap{i + 1}")
finally:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for the try/finally blocks, defer helps a lot.

@Lankou66
Lankou66 marked this pull request as draft July 28, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants